Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Laurentvallet issue#1980 (qc in ShUp) #2420

Open
wants to merge 44 commits into
base: develop
Choose a base branch
from

Conversation

LaurentPV
Copy link
Contributor

Allowing ShanoirUploader to execute quality-control check using datasets ms code before executing the import.

Check quality code externalized in a new QualityService class in datasets.

Same rules as quality check on server side.

Apply only if quality card is checkAtImport.

Returns qualityCardResult report if in error, warning or failed valid.

Update upload status in ERROR if quality check in error.

Copy link
Collaborator

@julien-louis julien-louis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no problems with the code, now I must do some tests

// Has quality check passed ?
if (qualityResult.hasError()) {
if (qualityResult != null && !qualityResult.isEmpty() && qualityResult.hasError()) {
// TODO : Delete newly created Examination ?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think deleting the exam would not fit to the frontend import workflow as the exam is created "independently"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was worried about the empty created exams if quality control is KO and the fact that the user may recreated a new exam each time he tries to reimport the data.

<version>3.2.2</version>
<exclusions>
<exclusion>
<groupId>*</groupId>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does that mean ? I guess it doesn't exclude everything does it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It excludes all dependencies from the dependency ms-datasets to avoid dependency loops

@julien-louis
Copy link
Collaborator

@LaurentPV as said in the chat, failing to check quality should not bypass it. It should result in stopping the import and prompting an explicit error message.

@julien-louis
Copy link
Collaborator

Hi @LaurentPV I'm vey sorry I take so much time to review this but could you lease merge develop to your branch there are 2 conflicts that I don't know how to fix.

Copy link
Collaborator

@julien-louis julien-louis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a bug when a study has no quality card, it blocks the import.

2024-11-12 11:35:17 ERROR [org.shanoir.uploader.action.ImportDialogOpener] [77] Cannot invoke "java.util.Collection.toArray()" because "c" is null
java.lang.NullPointerException: Cannot invoke "java.util.Collection.toArray()" because "c" is null
	at java.base/java.util.ArrayList.addAll(ArrayList.java:752)
	at org.shanoir.uploader.action.ImportDialogOpener.getAllQualityCards(ImportDialogOpener.java:349)
	at org.shanoir.uploader.action.ImportDialogOpener.getStudiesWithStudyCards(ImportDialogOpener.java:119)
	at org.shanoir.uploader.action.ImportDialogOpener.openImportDialog(ImportDialogOpener.java:60)
	at org.shanoir.uploader.nominativeData.CurrentNominativeDataController$1.mouseClicked(CurrentNominativeDataController.java:108)
	at java.desktop/java.awt.AWTEventMulticaster.mouseClicked(AWTEventMulticaster.java:278)
private List<QualityCard> getAllQualityCards(List<Study> studies) throws Exception {
		List<QualityCard> qualityCards = new ArrayList<QualityCard>();
		for (Iterator<Study> iterator = studies.iterator(); iterator.hasNext();) {
			Study study = (Study) iterator.next();
				qualityCards.addAll(shanoirUploaderServiceClient.findQualityCardsByStudyId(study.getId()));
		}
		return qualityCards;
	}

@julien-louis
Copy link
Collaborator

Also I wonder what is the meaning of this field that shows only auto check quality cards :
image

Does this mean you can't have several quality cards checks on imports ? I think the back checks every quality card that is set as auto-check.

@julien-louis
Copy link
Collaborator

  1. Sorry I can't make the uploader to block an import. The import goes well with my quality card here :
    image

@michaelkain michaelkain changed the title Laurentvallet issue#1980 Laurentvallet issue#1980 (qc in ShUp) Nov 13, 2024
@michaelkain
Copy link
Contributor

to see by Laurent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants